Skip to content

Conversation

@fabianofranz
Copy link
Member

Fixes #1349

@fabianofranz
Copy link
Member Author

@smarterclayton Take a look.
One note: I'm displaying context name AND project name, but not server and user. From the config file I don't know which username I am, one idea is to fetch it with a call to Users().Get(~), but it would make osc project <context> call the server. And I think just printing the authInfo name does not make much sense (authInfo.name can be the username, but not necessarily will be).
Any preference?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure what your comment meant, but make this an if block "if currentProject != currentContext { Using project %v from context named %v } else { Using project %v }"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@smarterclayton
Copy link
Contributor

If they provided a username show it, otherwise don't. We may want to store that info in the context eventually but for now it's fine to not have it.

----- Original Message -----

@smarterclayton Take a look.
One note: I'm displaying context name AND project name, but not server and
user. From the config file I don't know which username I am, one idea is to
fetch it with a call to Users().Get(~), but it would make osc project <context> call the server. And I think just printing the authInfo name
does not make much sense (authInfo.name can be the username, but not
necessarily will be).
Any preference?


Reply to this email directly or view it on GitHub:
#1405 (comment)

@fabianofranz fabianofranz force-pushed the issues_1349 branch 2 times, most recently from 145bfe7 to 7174f62 Compare March 20, 2015 23:20
@fabianofranz
Copy link
Member Author

@smarterclayton All comments addressed. I'm showing the server, but never the username - they are not supposed to provide --username to this command (I have a TODO to de-register --username and --password for every command other than login).

@fabianofranz
Copy link
Member Author

AND looks like stuff changed and I have to rebase. :)

@fabianofranz
Copy link
Member Author

Rebased.

@fabianofranz fabianofranz force-pushed the issues_1349 branch 3 times, most recently from 360ddaa to e142056 Compare March 23, 2015 15:08
@fabianofranz
Copy link
Member Author

@smarterclayton Fixed the latest comments.

@smarterclayton
Copy link
Contributor

Let me try it out.

@smarterclayton
Copy link
Contributor

$ osc project foo
Using server "https://localhost:8443".
Now using project "foo" from context named "b4596c40-d1b5-11e4-b7f8-7831c1b76042".

Should be using "foo" as the context name (by default) and the message should be

"Now using project "foo" on server https://localhost:8443"

because context name == project name. Don't need two lines for server.

----- Original Message -----

@smarterclayton Fixed the latest comments.


Reply to this email directly or view it on GitHub:
#1405 (comment)

@smarterclayton
Copy link
Contributor

$ osc project foo
Using server "https://localhost:8443".
Now using project "foo" from context named "b4596c40-d1b5-11e4-b7f8-7831c1b76042".

$ osc project bar
Using server "https://localhost:8443".
Now using project "bar" from context named "fc10fe66-d1b5-11e4-b654-7831c1b76042".

$ osc project foo
Using server "https://localhost:8443".
Now using project "foo" from context named "fd1e107a-d1b5-11e4-9b48-7831c1b76042".

The last one should be using the context name from the first time (which should be foo, but it looks like it's generating a new one every time)

  1. Clean slate (no contexts)
  2. osc project foo
  3. Have one context "foo" pointing to project "foo"
  4. osc project bar
  5. Have two contexts "foo" and "bar" pointing to those projects
  6. osc project bar --server=
  7. should create a new context "bar-" pointing to project bar on that server (if i have a single cluster with that server name)
  8. osc project bar --cluster=
  9. should create a new context "bar-" pointing to project bar on that server
  10. osc project bar --context=
  11. if context name does not exist, it should create it pointing to that project. otherwise, if the context already exists pointing to a different project and different server, it should give me an error.

I don't think you need to implement 6-9 here, but eventually we'll need to be able to do that.

----- Original Message -----

$ osc project foo
Using server "https://localhost:8443".
Now using project "foo" from context named
"b4596c40-d1b5-11e4-b7f8-7831c1b76042".

Should be using "foo" as the context name (by default) and the message should
be

"Now using project "foo" on server https://localhost:8443"

because context name == project name. Don't need two lines for server.

----- Original Message -----

@smarterclayton Fixed the latest comments.


Reply to this email directly or view it on GitHub:
#1405 (comment)

@fabianofranz
Copy link
Member Author

@smarterclayton Most issues have been addressed, please give it a try.

I also integrated the context identifier generator between project and login that will now use the same logic. Will try to generate the best identifier possible with the information available, removing unwanted characters like :./. Will result into something like this (in order, subsequent ones will be used when previous ones are taken):

foo
foo-localhost
foo-localhost-8443
foo-localhost-8443-username
foo-localhost-8443-username-0
foo-localhost-8443-username-1
(...)
UUID if nothing else works

@smarterclayton
Copy link
Contributor

When would uid be necessary?

On Mar 24, 2015, at 12:45 AM, Fabiano Franz notifications@github.com wrote:

@smarterclayton Most issues have been addressed, please give it a try.

I also integrated the context identifier generator between project and login that will now use the same logic. Will try to generate the best identifier possible with the information available, removing unwanted characters like :./. Will result into something like this (in order, subsequent ones will be used when previous ones are taken):

foo
foo-localhost
foo-localhost-8443
foo-localhost-8443-username
foo-localhost-8443-username-0
foo-localhost-8443-username-1
(...)
UUID if nothing else works

Reply to this email directly or view it on GitHub.

@fabianofranz
Copy link
Member Author

Most likely never, but if every other 104 context identifiers we tried to generate are taken, we need something to be the context identifier.

----- Original Message -----

When would uid be necessary?

On Mar 24, 2015, at 12:45 AM, Fabiano Franz notifications@github.com
wrote:

@smarterclayton Most issues have been addressed, please give it a try.

I also integrated the context identifier generator between project and
login that will now use the same logic. Will try to generate the best
identifier possible with the information available, removing unwanted
characters like :./. Will result into something like this (in order,
subsequent ones will be used when previous ones are taken):

foo
foo-localhost
foo-localhost-8443
foo-localhost-8443-username
foo-localhost-8443-username-0
foo-localhost-8443-username-1
(...)
UUID if nothing else works

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub:
#1405 (comment)

@smarterclayton
Copy link
Contributor

I think in that case it would be ok to bail and report an error to the user that they need to provide a context name on the CLI

----- Original Message -----

Most likely never, but if every other 104 context identifiers we tried to
generate are taken, we need something to be the context identifier.

----- Original Message -----

When would uid be necessary?

On Mar 24, 2015, at 12:45 AM, Fabiano Franz notifications@github.com
wrote:

@smarterclayton Most issues have been addressed, please give it a try.

I also integrated the context identifier generator between project and
login that will now use the same logic. Will try to generate the best
identifier possible with the information available, removing unwanted
characters like :./. Will result into something like this (in order,
subsequent ones will be used when previous ones are taken):

foo
foo-localhost
foo-localhost-8443
foo-localhost-8443-username
foo-localhost-8443-username-0
foo-localhost-8443-username-1
(...)
UUID if nothing else works

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub:
#1405 (comment)


Reply to this email directly or view it on GitHub:
#1405 (comment)

@smarterclayton
Copy link
Contributor

LGTM [merge] - follow ups opened as issues.

@fabianofranz
Copy link
Member Author

@smarterclayton Just fixed your latest comment.

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1283/) (Image: devenv-fedora_1126)

@openshift-bot
Copy link
Contributor

Evaluated for origin up to 4318ad1

openshift-bot pushed a commit that referenced this pull request Mar 25, 2015
@openshift-bot openshift-bot merged commit 0b4bb0a into openshift:master Mar 25, 2015
jboyd01 pushed a commit to jboyd01/origin that referenced this pull request Oct 17, 2017
…service-catalog/' changes from 3aacfedec6..aa27078754

aa27078754 origin build: add origin tooling
bcf37fd 0.1.0-rc2 chart updates (openshift#1410)
4ab0a0a add back 'Processing' message for instance deletion (openshift#1332)
0ecbcb1 Update logs for Cluster service plans. (openshift#1389)
8b491ef Fix a quoting nit (openshift#1400)
63685e4 add orphan mitigation-specific conditions for instances (openshift#1378)
adee662 Updated missed fields in service and plan specs (openshift#1406)
2095919 Handle default plan setting when using k8s names (openshift#1405)
607ba66 Document rbacEnable. (openshift#1404)
268294e Adding rbac definition for v1 api endpoint. (openshift#1284)
103288d differentiate between failed updates and provisions during deletion (openshift#1383)
eba8ba4 enable API aggregation and Service Catalog RBAC on Jenkins (openshift#1333)
5a93315 Validate relistDuration is non-negative (openshift#1395)
e279d21 Fix log messages for secrets (openshift#1385)
87fa8c9 fix status update when starting orphan mitigation (openshift#1372)
11f18f3 Switch to wget for integration apiserver checks (openshift#1384)
8c44a7d update OSB client to 2.13 (openshift#1392)
e64bbd1 default plan admission controller: filter list of service plans/service classes by the class name (openshift#1351)
6648c0e Check field names. Fix issue 1291 (openshift#1379)
5319841 update comment for instance generation check (openshift#1382)
7d5823f remove internal poll method (openshift#1381)
07d3068 Rework the logging for controller_instance. (openshift#1371)
5f4ca01 address PR comment as a followup (openshift#1380)
485d5e6 Add support for specifying plan using K8S names. (openshift#1377)
662bba8 Log number of secret keys created for binding credential (openshift#1375)
8ad6a31 Move controller constants into correct files (openshift#1373)
7bd66dd Adding type to log. (openshift#1339)
1ce5c4d Remove k8s/k8s dependency (openshift#1355)
b458323 Adding log formatting for BindingController. (openshift#1352)
275eb11 rename test variables to be consistent (openshift#1315)
ffd6b8b travis: skip cleanup before deploy (openshift#1368)
d5ecc04 fix travis tag checker (openshift#1365)
2cae0ee Minor updates to README (openshift#1360)
REVERT: 3aacfedec6 carry: Set external plan name for service-catalog walkthrough
REVERT: 3ec9e5b07a origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: aa2707875461dd51be3731b1d94b5cfc3b9a3976
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018
* Handle default plan setting when using k8s names

* address golint issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

osc project doesn't check context first

3 participants